Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARM:Add Jtag to Dormant sequence in case the target has both jtag and swd #1959

Merged

Conversation

mean00
Copy link

@mean00 mean00 commented Oct 13, 2024

Detailed description

  • The current Dormant to SWD code manages correctly the case where SWD is default/the only interface
  • If the chip has both SWD & Jtag and default is Jtag, it lacks the Jtag->dormant sequence and does not work
  • The proposed patch adds that sequence

For what i could see, it still works with RP2040 and older Arm chips i have (STM32F1/GD32F3)
Tested on a chip with both Jtag & SWD/Default jtag + Debug Protocol v3, it works with the change, doesnt without

NB: Not completely sure the 8 trailer '1' bits are really needed, it works fine with, was there in the demo code

Your checklist for this pull request

Closing issues

@mean00 mean00 force-pushed the mean00/add_jtag_to_dormant_sequence branch from ee984ad to b79fc11 Compare October 13, 2024 07:31
Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having picked through the spec to check this, this all looks good. With our only review notes (stylistic) fixed, please rebase this on main and we'll get it merged.

We checked the specification where it talks about how to step through the various state machines, and per the spec, those 8 high cycles are required:

image

src/target/adiv5_swd.c Outdated Show resolved Hide resolved
src/target/adiv5_swd.c Outdated Show resolved Hide resolved
@dragonmux dragonmux added this to the v2.0 release milestone Oct 23, 2024
@dragonmux dragonmux added Bug Confirmed bug New Target New debug target labels Oct 23, 2024
@mean00 mean00 force-pushed the mean00/add_jtag_to_dormant_sequence branch 3 times, most recently from 307d8a3 to 54e6cf2 Compare October 24, 2024 06:18
@mean00
Copy link
Author

mean00 commented Oct 24, 2024

I think i fixed all the comments & rebased against main.

Thank you.

Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all LGTM, merging once it's re-rebased on main (apologies, we just merged #1941, hence). Thank you for the contribution!

Edit: In the interests of seeing this merged, we have done that final rebase for you and will presently merge this PR.

@dragonmux dragonmux force-pushed the mean00/add_jtag_to_dormant_sequence branch from 54e6cf2 to 2de7560 Compare October 24, 2024 07:06
@dragonmux dragonmux merged commit 2de7560 into blackmagic-debug:main Oct 24, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bug New Target New debug target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants